Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix ci test error introduced by (#9781) #9838

Merged
merged 6 commits into from
Feb 8, 2023
Merged

Conversation

process852
Copy link
Contributor

  1. 将原来使用fabs判断误差改用为flow.allclose函数
  2. 删除了重复的测试单元test_multiplicative_lr(原来写了2个一样的)
  3. 提高了部分flow.allclose中rtol的值,解决概率性不通过的问题add swa_utils modules #9781 (comment)

math.fabs(target[epoch] - param_group["lr"]) < 1e-5,
flow.allclose(
flow.tensor(target[epoch]),
flow.tensor(param_group["lr"]),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里用 np.allclose 吧,不用创建新的 tensor,同理还有 145 行

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的

@@ -202,7 +201,9 @@ def _test_averaged_model(self, net_device, swa_device):
averaged_dnn.update_parameters(dnn)

for p_avg, p_swa in zip(averaged_params, averaged_dnn.parameters()):
self.assertTrue(flow.max(flow.abs(p_avg.cpu() - p_swa.cpu())).item() < 1e-5)
self.assertTrue(
flow.allclose(p_avg.cpu(), p_swa.cpu(), atol=1e-5, rtol=1e-4)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里需要 .cpu() 吗

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

这里是是需要的,在test_averaged_model_all_devices中进行了不同设备之间的测试
image

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的

@mergify mergify bot merged commit 0cc4a56 into Oneflow-Inc:master Feb 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants